Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hotfix UserRole get users config. #849

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Maras0830
Copy link

default user-role map table should not in config auth.users table.

@Zizaco
Copy link
Owner

Zizaco commented Nov 13, 2017

@Maras0830 Sorry, can you explain what exactly is the issue that you are having?

@Maras0830
Copy link
Author

Maras0830 commented Nov 15, 2017

@Zizaco
in entrust.php

    /*
    |--------------------------------------------------------------------------
    | Application User Model
    |--------------------------------------------------------------------------
    |
    | This is the User model used by Entrust to create correct relations.
    | Update the User if it is in a different namespace.
    |
    */
    'user' => 'App\User',

If my project have multiple guard identities, when I want to get role - users
that will return laravel framework config auth user table .. not the packages user table ...

because in EntrustRoleTrait.php

    /**
     * Many-to-Many relations with the user model.
     *
     * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany
     */
    public function users()
    {
        return $this->belongsToMany(Config::get('auth.providers.users.model'), Config::get('entrust.role_user_table'), Config::get('entrust.role_foreign_key'), Config::get('entrust.user_foreign_key'));
    }

so I push this commit 336382e.
we should be use this packages config user table, not framework default auth user table.

The relation of multiple identities is not necessarily the same as auth user.

vinayakkulkarni added a commit to vinayakkulkarni/entrust that referenced this pull request Nov 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants